in my work on OSC sequences i realized... I don't think caching actually does anything for the output engine any longer. The cached sequence is never emitted because OutputStateMachineEngine::ActionPassThroughString does nothing.

Which makes sense, actually. Since ConPTY became passthrough mode, there's no reason to cache. We just yeet everything through.

I can't do a full review, but I just wanted to note that this part of the code likely needs more work. When the SixelParser calls this method, it needs to know that it's processing the very last character, and not just a buffer that includes the last character.

So I expect that state now needs to be tracked in the SixelParser itself. Where we're looping over the buffer, we may need something like this:

const auto lastCharacterBuffer = _stateMachine.IsProcessingLastCharacter();
for (auto = 0; i < str.length(); i++)
{
    _processingLastCharacter = lastCharacterBuffer && (i == str.length() - 1);
    _parseCommandChar(str[i]);
}

And wherever we're calling IsProcessingLastCharacter we would instead use this local _processingLastCharacter flag instead.

I'm not positive, but I think the SixelParser is the only place that is still using IsProcessingLastCharacter.

I came back to this today to address your comment. Is tracking lastCharacterBuffer = _stateMachine.IsProcessingLastCharacter() necessary at all at that point? Wouldn't just i == str.length() - 1 alone be a good enough heuristic? It'd be neat, because it would let us fully decouple the sixel- from the VT-parser.

I've pushed a commit based on this. Since the comments hinted at animations, I tested it with mpv --really-quiet --vo=sixel and it seemed to work fine without stutter and hitches.

Although... this doesn't test animations with the palette color. Do you have a pre-existing test for that? Otherwise, I'd try and write my own.

That's what I had hoped initially, but I got the impression that you were not actually sending through the full buffer all the time (I may have misunderstood how it now works though). And the risk is that it may trigger an unwanted update that could produce flickering in things like video sequences (this is exacerbated by the fact that a lot of image libraries request an unnecessary background fill, so if we flush before the full image has been received you can end up seeing a large block of some random background color).

That said, it is just a heuristic, and there was always a risk of flickering, so I'm happy to go with whatever is easiest and see if anyone complains. Modern apps can always work around flickering with synchronized update mode if necessary.

Although... this doesn't test animations with the palette color. Do you have a pre-existing test for that?

Sorry, I didn't initially see this message. There's a palette animation test in my initial PR here (plasma.py): #17421 (comment)

You can also confirm that progressive output is working with this script:
https://github.com/hackerb9/vt340test/blob/main/jerch/endless.sh

It's not that smooth on Windows Terminal, I think because of conpty buffering, but you should still see it updating continuously.

Thanks for the tests!

https://github.com/hackerb9/vt340test/blob/main/jerch/endless.sh

Hmm weird, that script doesn't work for me at all.

I may be misunderstanding the intention here, but this looks wrong to me. I would have expected the condition to be something like:

runEnd != end && !_isEscape(*runEnd) && (_isC0Code(*runEnd) || _isDcsPassThroughValid(*runEnd))

i.e. it's not an escape, but is valid for DCS passthrough, which means either C0 or in the passthrough range.

That said, I'm not sure its even worth checking for valid DCS characters at this level if the goal is to pass through as much as possible in a single run. I suspect the existing DCS string handlers are already ignoring any characters that are out of range, so it's probably safe to pass through everything. And I wouldn't be surprised if future DCS sequences may actually need to receive all of those values anyway.

But the bigger problem I see here is that we don't appear to be checking for all the terminators. In addition to ESC, a string sequence can also be terminated by CAN, SUB, or any of the C1 controls. And to complicate matters further, the DCS string handlers expect the string that they receive to always be terminated with an ESC, regardless of the actual terminating character, so you can't just rely on the ESC being in the original buffer.

Btw, there's a sixel test that covers the different terminators here:
https://github.com/hackerb9/vt340test/blob/main/j4james/unexpected.sh
Expected output looks like this:
https://github.com/hackerb9/vt340test/blob/main/j4james/unexpected.png